Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drive #3

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Drive #3

wants to merge 5 commits into from

Conversation

marialramosc
Copy link
Member

New libraries for motors without BNO

marialramosc and others added 3 commits March 5, 2024 16:48
The first main code displays the angle detected plus the angle the robot should move, but the angle detected its not the expected one
private:
int motorSpeed;
int pin1;
int pin2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reformat variable names following the convention:

int pin_speed_;
int pin_1_;

public:
motor motor1;
motor motor2;
motor motor3;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the convention, class names should be in uppercase and member variables in snake case with a trailing underscore:

Motor motor_1_;

#include "motor.h"
#include "Arduino.h"

class drive {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constants could be added as public members in a Constants.h file and used directly in the init function.

Serial.begin(9600);

// initialize drive system
drive robotDrive(m1Speed, m1P1, m1P2, m2Speed, m2P1, m2P2, m3Speed, m3P1, m3P2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intialization of this object should be global


float m1 = sin(((60 - degree) * PI / 180));
float m2 = sin(((180 - degree) * PI / 180));;
float m3 = sin(((300 - degree) * PI / 180));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could encapsulte setting the speeds of the motors in a local function, for example:

motor1.setSpeed(m1*speed);

digitalWrite(pin1, LOW);
digitalWrite(pin2, LOW);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local function for set the motor speed example:

void Motor::SetSpeed(int pwm){
    int speed = abs(pwm);
    //set motor speeds
    analogWrite(getMotorSpeed(), speed);
    if (pwm >= 0) {
        motorFrontward();
    } else {
        motorBackward();
}

Copy link
Member

@afr2903 afr2903 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've seen the robot and this code is functional, just refactor the variable and function names, and more importantly make the object intialization globlal in the main .ino file

Copy link
Member

@afr2903 afr2903 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution was working, please refactor following the convention for naming

yaw = (double)orientationData.orientation.x;
// convert yaw angle to range -180 to 180
yaw = (yaw > 0) ? (360 - yaw) : yaw;
yaw = (yaw > 180) ? (yaw - 360) : yaw;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to make this expression in a single line.

Copy link
Member

@afr2903 afr2903 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll aprove the PR for you to merge the required changes into main and change how we work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants